fix: --web opens a browser tab for every PR when submitting a multi-branch stack#135
fix: --web opens a browser tab for every PR when submitting a multi-branch stack#135mvanhorn wants to merge 2 commits into
Conversation
|
@mvanhorn Thanks for your help! Glad you are finding this useful. |
There was a problem hiding this comment.
Pull request overview
Adjusts gh stack submit --web to only open browser tabs for newly created PRs (not updated/adopted ones), aligning behavior with the intended UX for multi-branch stacks.
Changes:
- Restricts
--webURL collection to newly created PRs viacollectSubmitPRURL, and updates CLI help text accordingly. - Updates README flag documentation for the refined
--webbehavior. - Adds unit tests to verify which PR actions contribute URLs for browser opening.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates --web flag documentation to “newly created PRs” only. |
| cmd/submit.go | Stops collecting URLs for updated/adopted PRs; collects URLs only for successfully created PRs. |
| cmd/submit_internal_test.go | Adds unit tests covering URL collection behavior for --web across create/update/adopt/failure/dry-run cases. |
| cmd/restack.go | Adds early return for non-conflict rebase errors (plus suggested improvement to error context). |
| cmd/adopt.go | Refactors cycle detection to walk parent links directly (not described in PR scope). |
| .osc-metadata/sync.json | Adds fork-sync metadata file (appears unrelated to feature change). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if rebaseErr != nil && !g.IsRebaseInProgress() { | ||
| return rebaseErr | ||
| } |
| // Check for cycles by walking configured parent links directly. A broken | ||
| // parent can disconnect branches from the trunk-rooted tree while their | ||
| // stackParent links still form a cycle. | ||
| seen := map[string]bool{} | ||
| for current := parent; current != trunk; { | ||
| if current == branchName { | ||
| return errors.New("cannot adopt: would create a cycle") | ||
| } | ||
| if seen[current] { | ||
| return errors.New("cannot adopt: parent chain contains a cycle") | ||
| } | ||
| seen[current] = true | ||
|
|
||
| parentNode := tree.FindNode(root, parent) | ||
| if parentNode != nil { | ||
| for _, ancestor := range tree.GetAncestors(parentNode) { | ||
| if ancestor.Name == branchName { | ||
| return errors.New("cannot adopt: would create a cycle") | ||
| } | ||
| next, parentErr := cfg.GetParent(current) | ||
| if parentErr != nil { | ||
| break | ||
| } | ||
| current = next | ||
| } |
There was a problem hiding this comment.
Yeah, this looks like it came from an attempted fix for #115.
| { | ||
| "fork_synced_at": "2026-07-10T09:11:21.236884+00:00", | ||
| "commits_behind_before_sync": 24, | ||
| "action_taken": "synced" | ||
| } No newline at end of file |
There was a problem hiding this comment.
@mvanhorn Out of curiosity, what tool generates this?
boneskull
left a comment
boneskull
left a comment
There was a problem hiding this comment.
- Split out change to
adopt.go - Remove metadata
- Handle error as suggested
- Restore cmd/adopt.go to upstream; the cycle-detection refactor (from a boneskull#115 fix attempt) is out of scope here and will land separately. - Remove the accidentally committed .osc-metadata/sync.json. - Wrap the non-conflict rebase error in the main-repo restack flow with branch/base context, matching the worktree-branch path.
|
All three sorted in 8b5e25e. The .osc-metadata/sync.json was from a local fork-sync helper I run to keep my fork current with upstream, committed by accident, so it is gone now. I also reverted cmd/adopt.go back to upstream, since that cycle-detection change came from a local #115 attempt and belongs in its own PR rather than this one. And the main-repo rebase path now wraps its error with the branch and base, matching the worktree branch above. |
Summary
Implement the maintainer's option 1 combined with option 5's URL-printing half, which the code already satisfies: only append to
prURLsin theprActionCreatenon-adopted path (theCreated PR #%dbranch), and stop appending in theprActionUpdateand adoption paths (bothprActionAdoptand theadoptedresult ofexecutePRCreate), since those PRs already existed and their URLs are already printed inline vias.Hyperlink/PRURLin the status lines. Update the--webflag help string incmd/submit.gofrom "open created/updated PRs in web browser" to reflect the new behavior ("open newly created PRs in web browser"), and update the matching--webrow in README.md's flag table (line ~290). Add/extend unit tests incmd/submit_internal_test.gocovering which actions collect URLs for opening.Why this matters
gh stack submit --webopens a separate browser tab for every PR that was created, adopted, or updated in the stack, so a stack of N branches carpet-bombs the user with N tabs. The maintainer (repo owner, who filed the issue) pinpointed the code: incmd/submit.go, every successfulprActionUpdate,prActionAdopt, andprActionCreatebranch appendsghClient.PRURL(...)to aprURLsslice, and a final loop callsbrowser.Browse()on each URL. The issue lists five candidate directions and names option 1 ("only open newly created PRs - skip updated ones, since the user already knows about those") as the least disruptive; a follow-up comment adds that removing--webentirely would also be acceptable.See #44.
Testing
--webwith a create failure on one branch: failed branch contributes no URL, remaining created PRs still open.Fixes #44